Skip to content

Fix exception because of premature handle closing in V2 forkers#1842

Merged
jasagredo merged 1 commit intomainfrom
js/forkers-2
Feb 10, 2026
Merged

Fix exception because of premature handle closing in V2 forkers#1842
jasagredo merged 1 commit intomainfrom
js/forkers-2

Conversation

@jasagredo
Copy link
Copy Markdown
Contributor

@jasagredo jasagredo commented Jan 27, 2026

Manifested here: https://github.com/IntersectMBO/ouroboros-consensus/actions/runs/21365715331/job/61496793074?pr=1841

Finally I know what is going on and it is quite convoluted:

  • We have three handles in the forker registry (with theirs resource ID (rID for short)) A(2), B(4), C(5). They belong to these branches:
.. 565 -- -617 (A,2)
      \-- -617 (B,4) -- -611 (C,5)
  • A(2) is there only because we forgot to close it.
  • When transferring handles to the LedgerDB, we move the three of them, and they get new rIDs: A(2 -> 6), B(4 -> 7), C(5 -> 8), BUT we only update the rIDs known to the handles for B and C, so they think they are the previous resource
  • This means
    • A is allocated at rID 6 and it is not made aware that it was transferred
    • B is allocated at rID 7 but it is told its new rID is instead 6
    • C is allocated at rID 8 but it is told its new rID is instead 7
  • Then we select some fork that discards C, so we close it.
  • Closing C will release resource 7 as that is the rID it thinks it belongs to. But that actually closes B.
  • Then when we try to access B it will be closed, AND it will report in the exception that 6 was closed because that is what it thinks is its rID. It was 7 but it didn't know that.

So albeit the logic being quite complicated, the solution is in fact making sure that we close A when we discard that fork.

@jasagredo
Copy link
Copy Markdown
Contributor Author

I'm thinking now that maybe we could even share the same ledgerseq between the LedgerDB and the forker, and commit the whole thing, then closing only the leftovers from the LedgerDB. This would remove the fromJust construct

@jasagredo jasagredo self-assigned this Jan 29, 2026
@jasagredo
Copy link
Copy Markdown
Contributor Author

I'm thinking now that maybe we could even share the same ledgerseq between the LedgerDB and the forker, and commit the whole thing, then closing only the leftovers from the LedgerDB. This would remove the fromJust construct

Actually we can't benefit from this because the LedgerDB might be pruned in the meantime, so we will have to do the same dance.

github-merge-queue bot pushed a commit that referenced this pull request Feb 3, 2026
Part of #1842 that I would like to merge separatedly.
@jasagredo jasagredo force-pushed the js/forkers-2 branch 2 times, most recently from e116f59 to 6fb6220 Compare February 4, 2026 08:31
@jasagredo jasagredo changed the base branch from main to js/best-ghc February 10, 2026 12:01
Base automatically changed from js/best-ghc to main February 10, 2026 13:35
@jasagredo jasagredo added this pull request to the merge queue Feb 10, 2026
Merged via the queue into main with commit c2b0422 Feb 10, 2026
20 of 21 checks passed
@jasagredo jasagredo deleted the js/forkers-2 branch February 10, 2026 16:47
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Consensus Team Backlog Feb 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants